Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make integer exponentiation methods unstably const #68978

Merged
merged 4 commits into from
Feb 20, 2020

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Feb 9, 2020

cc #53718

This makes the following inherent methods on integer primitives into unstable const fn:

  • pow
  • checked_pow
  • wrapping_pow
  • overflowing_pow
  • saturating_pow
  • next_power_of_two
  • checked_next_power_of_two
  • wrapping_next_power_of_two

Only two changes were made to the implementation of these methods. First, I had to switch from the ? operator, which is not yet implemented in a const context, to a try_opt macro. Second, next_power_of_two was using ops::Add::add (see the first commit) to "get overflow checks", so I switched to #[rustc_inherit_overflow_checks]. I'm not quite sure why the attribute wasn't used in the first place.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 9, 2020
@rust-highfive

This comment has been minimized.

@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Feb 9, 2020

So I found #45754, which originally used #[rustc_inherit_overflow_checks] for next_power_of_two but switched to ops::Add::add. Is there a rule against adding that attribute in new places? The test added in that PR passes with my changes. cc @scottmcm.

@ecstatic-morse ecstatic-morse changed the title Make integer exponentiation methods const Make integer exponentiation methods unstably const Feb 9, 2020
@Centril
Copy link
Contributor

Centril commented Feb 10, 2020

r? @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Feb 10, 2020

lgtm, we should remove the macro at some point, but that's not really a problem to leave around until someone discovers it.

I also don't know about the panic situation in that function. r=me with @scottmcm 's approval on that panic "change"

@scottmcm
Copy link
Member

scottmcm commented Feb 20, 2020

I have no horse in this race; I added it because I was asked to do so by @kennytm in #45754 (comment)

AFAIK that's just a matter of "use fewer #[rustc*] things where possible", so if it's not possible in this case and compiler people are fine with it, I'm fine with it.

So I think that means
@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Feb 20, 2020

📌 Commit 7fe5eaf has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 20, 2020
…-obk

Make integer exponentiation methods unstably const

cc rust-lang#53718

This makes the following inherent methods on integer primitives into unstable `const fn`:
- `pow`
- `checked_pow`
- `wrapping_pow`
- `overflowing_pow`
- `saturating_pow`
- `next_power_of_two`
- `checked_next_power_of_two`
- `wrapping_next_power_of_two`

Only two changes were made to the implementation of these methods. First, I had to switch from the `?` operator, which is not yet implemented in a const context, to a `try_opt` macro. Second, `next_power_of_two` was using `ops::Add::add` (see the first commit) to "get overflow checks", so I switched to `#[rustc_inherit_overflow_checks]`. I'm not quite sure why the attribute wasn't used in the first place.
This was referenced Feb 20, 2020
bors added a commit that referenced this pull request Feb 20, 2020
Rollup of 5 pull requests

Successful merges:

 - #68705 (Add LinkedList::remove())
 - #68945 (Stabilize Once::is_completed)
 - #68978 (Make integer exponentiation methods unstably const)
 - #69266 (Fix race condition when allocating source files in SourceMap)
 - #69287 (Clean up E0317 explanation)

Failed merges:

r? @ghost
@bors bors merged commit d96951f into rust-lang:master Feb 20, 2020
@ecstatic-morse ecstatic-morse deleted the const-int-pow branch October 6, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants